Post-merge-review: extend allowlist with svg-tags on template-no-block-params-for-html-elements by johanrd · Pull Request #2689 · ember-cli/eslint-plugin-ember

johanrd · 2026-04-13T15:59:37Z

Summary

Adds svg-tags to the element allowlist alongside existing html-tags — so <circle as |x|> etc. are now flagged.
Documents accepted false negatives via new test cases: custom elements (<my-element as |x|>) and namespaced components (<NS.Foo as |x|>) aren't in the allowlists, so they're not flagged. Web-component namespace is open and can't be enumerated.
Minor comment cleanup in the rule.

Test plan

<div as |x|> / <section as |x|> / <ul as |x|> → flagged
<MyComponent as |x|> → valid (uppercase, not in allowlist)
<NS.Foo as |x|> → valid (dotted)
<my-element as |x|> → valid (accepted false negative — web component)
<div as |x|> where div is a local binding in GJS → valid (scope check)

NullVoxPopuli

I don't agree with the direction of this PR -- using a static list for html-tags is better because we don't actually have good hueristics for detecting if something is a component or not, since components can be defined as htmltags

NullVoxPopuli

Don't remove html-tags

johanrd · 2026-04-14T13:39:28Z

@NullVoxPopuli extending html-tags could patch some gaps, yes. It would miss SVG elements (<path>) and MathML elements (<mfrac>) — but those are solvable by adding svg-tags or similar packages alongside.

But what about Web Components? Their naming convention is lowercase-with-hyphen (<my-widget>), and by definition there's no finite enumerable list of them — the namespace is open to anyone publishing a component library. So <my-widget as |x|>{{x}}</my-widget> is unambiguously a bug, but no combination of tag-list packages can ever catch it.

The inverse approach sidesteps this entirely because the set of things that are Glimmer component invocations is small and bounded (uppercase first char, ., @, this., local scope) — everything else is "not a component" by what the runtime actually does.

Ember-template-lint's own implementation of these rules already reached this conclusion — https://github.com/ember-template-lint/ember-template-lint/blob/main/lib/helpers/is-angle-bracket-component.js uses exactly this inverse detection with no html-tags allowlist anywhere. The conservative approach is to go with the ember-template-lint implementation for now, no?

NullVoxPopuli · 2026-04-14T14:19:41Z

html-tags is not meant to have all tags, but is meant to give us confidence that the tags it does have are HTML -- there are separate packages for svg-tags and mathml-tags,.

web components

these cannot be known ahead of time, so we have to fallback to lower-case with hyphen check... however, do we care if we see something as a web-component?

if it's in scope, we know wheer it is. if it's not in scope, it's defined globally (html, svg, mathml, web-component), etc.

isAngleBracketComponent is flawed and only is possible of working in loosemode, we can onlyl suceeed through known lists and scope analysis.

johanrd · 2026-04-14T17:43:18Z

Thanks, for the pushback. You are probably right. I can revert to lists + scope (and add svg-tags / mathml-tags alongside?).

The only tradeoff/regression (as i understand it) is then in HBS mode, where <foo as |x|> is a silent false negative under the html-list approach?

Mode	Lists + scope	Inverse heuristic
GTS	caught via scope ✓	caught via lowercase check ✓
HBS	missed (empty scope + not in any list)	caught ✓

NullVoxPopuli · 2026-04-15T03:19:15Z

hbs should have scope enabled for <foo as |x|> -- there should be no tradeoff to using scope

johanrd · 2026-04-15T06:30:12Z

Investigated this through claude: The HBS parser seems to create an empty scope manager. From node_modules/ember-eslint-parser/src/parser/hbs-parser.js lines 70–81:

// Create an empty scope manager.
// For HBS, all locals are assumed to be defined at runtime,
// so we don't track variable references (no no-undef errors).
const scopeManager = eslintScope.analyze(
  {
    type: 'Program',
    body: [],  // ← empty — no bindings registered
    range: [0, code.length],
    loc: program.loc,
  },
  { range: true }
);

Block params from <foo as |x|> exist on GlimmerElementNode.blockParams in the AST, but they are not registered in the scope manager in HBS mode. context.getScope() returns a global scope with no child scopes and no template-defined variables.

In GTS/GJS mode the main parser uses buildGlimmerVisitors (via ember-estree's toTree) which does register block params in scope. HBS mode calls toTree(code, { templateOnly: true }) with no visitors, so scope stays empty.

Concretely, in HBS mode scope.references.some(ref => ref.identifier === node.parts[0]) is always false — the scope check in the current implementation is dead code for HBS files. The isHtmlElement heuristic is what carries the HBS case.

If scope support for block params is added to ember-eslint-parser/hbs in the future, the heuristic can be dropped. For now it's necessary unless hbs can live with the uncaught of course.

NullVoxPopuli · 2026-04-15T11:51:39Z

claude, you do it

johanrd · 2026-04-15T13:43:27Z

@NullVoxPopuli are you intending to

fix ember-eslint-parser/hbs to populate block-param scope? (i will not touch that, i think)
or explicitly accept HBS false negatives opposed to the implementation of ember-template-lint?

Which?

NullVoxPopuli · 2026-04-15T14:15:16Z

I'm not intending anything -- you are / are using a bot, so the bot should fix the hbs parser to include scope <3 🎉.

Please PR to ember-eslint-parser

johanrd · 2026-04-15T18:41:08Z

okay, thanks. (I, as a human, genuinely did not understand from your brevity)

See ember-tooling/ember-eslint-parser#189

Switched to html-tags + svg-tags + scope now. Custom elements and other non-listed lowercase tags are accepted false negatives. MathML skipped for now — mathml-tag-names@4 is ESM-only.

…h svg-tags Adds svg-tags to the html-tags allowlist so <circle as |x|> etc. are flagged alongside HTML elements. Documents accepted false negatives (custom elements, namespaced components) via new test cases.

NullVoxPopuli · 2026-04-15T19:33:42Z

We support only the ESM-only nodes. so... node 20.19+ and all other nodes that have require(esm) -- we can use ESM-only packages

…ames Extends the allowlist with MathML tag names. NullVoxPopuli confirmed the plugin can use ESM-only packages since the supported Node range (>=20.19) has require(esm). Adds an invalid test for `<mfrac as |num|>` and an invalid test for `<circle as |r|>` (SVG, via svg-tags).

johanrd · 2026-04-15T19:39:12Z

okay, thanks, didn't know. added now.

ember-tooling/ember-eslint-parser#189 populates block-param scope for <foo as |x|> in HBS mode, so the rule's scope check in lib/rules/template-no-block-params-for-html-elements.js no longer relies on an empty scope manager in .hbs files. No observable test change on master (all tests here use <template>...</template> / GJS mode where scope already worked), but makes the scope-based detection correct in HBS too.

NullVoxPopuli

tyty

@NullVoxPopuli

…classification) Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in ember-cli#2689 (template-no-block-params-for-html-elements). ## Why not heuristics An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the ember-cli#2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; <template><div /></template>`), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. ## What the util does `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`<my-element>` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. ## Rules migrated Four rules now share the util: - `template-no-empty-headings` — fixes the `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in ember-cli#2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines.

@NullVoxPopuli

…classification) Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in ember-cli#2689 (template-no-block-params-for-html-elements). ## Why not heuristics An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the ember-cli#2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; <template><div /></template>`), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. ## What the util does `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`<my-element>` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. ## Rules migrated Four rules now share the util: - `template-no-empty-headings` — fixes the `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in ember-cli#2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines.

@NullVoxPopuli

…classification) Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in ember-cli#2689 (template-no-block-params-for-html-elements). ## Why not heuristics An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the ember-cli#2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; <template><div /></template>`), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. ## What the util does `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`<my-element>` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. ## Rules migrated Four rules now share the util: - `template-no-empty-headings` — fixes the `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in ember-cli#2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines.

@NullVoxPopuli

…classification) Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in ember-cli#2689 (template-no-block-params-for-html-elements). ## Why not heuristics An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the ember-cli#2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; <template><div /></template>`), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. ## What the util does `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`<my-element>` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. ## Rules migrated Four rules now share the util: - `template-no-empty-headings` — fixes the `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in ember-cli#2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines.

@NullVoxPopuli

…+ scope) Replace the inline PascalCase-regex isComponentInvocation heuristic with the lists + scope pattern established by @NullVoxPopuli in ember-cli#2689 and canonicalised by ember-cli#2724. The new lib/utils/is-native-element.js mirrors ember-cli#2724's util byte-for-byte so the two PRs can land in either order without conflict — whichever lands first introduces the file; the other rebases to a no-op on it. Heuristic approaches (PascalCase regex, `tag.includes('.')`, etc.) were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components in GJS/GTS when the name is shadowed by a scope binding (`const div = MyComponent; <div />`). Scope analysis catches this; regex heuristics don't. Call site now uses `!isNativeElement(node, sourceCode)` directly to match the canonical usage pattern in ember-cli#2724's migrated rules.

…+ scope) Remove the inline PascalCase-regex isComponentInvocation heuristic in favor of the lists + scope pattern from ember-cli#2689 / ember-cli#2724. The new lib/utils/is-native-element.js mirrors ember-cli#2724's util byte-for-byte so the two PRs can land in either order without conflict. evaluateChild / evaluateChildren now thread sourceCode through the recursion so the scope check has access to the enclosing template's bindings. Heuristic approaches were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components when shadowed by scope bindings. Treating custom elements as opaque (the same as components) is a behavior improvement — matches ember-cli#2724's convention.

@NullVoxPopuli

…classification) Adds `lib/utils/is-native-element.js` — a shared util for the recurring "is this a native HTML/SVG/MathML element or a component?" question. Uses the authoritative `html-tags` + `svg-tags` + `mathml-tag-names` packages plus scope analysis, mirroring the pattern established by @NullVoxPopuli in ember-cli#2689 (template-no-block-params-for-html-elements). An earlier draft of this PR used a lexical heuristic (PascalCase / `@` / `this.` / dot / `::`). That approach was rejected in the ember-cli#2689 discussion — a lowercase tag CAN be a component in GJS/GTS when its name is bound in scope (`const div = MyComponent; <template><div /></template>`), so heuristics produce both false positives (web components misclassified as native) and false negatives (scope-shadowed tags misclassified as native). Lists + scope handles both correctly. `isNativeElement(node, sourceCode)` returns true iff: - The tag name is in the HTML/SVG/MathML authoritative lists, AND - The tag name is NOT a scope-bound identifier. Returns false for components (PascalCase, dotted, @-prefixed, etc. — none of those tag-name shapes appear in the lists), custom elements (`<my-element>` — accepted false negative; web component namespace is open-ended), and scope-bound identifiers. Four rules now share the util: - `template-no-empty-headings` — fixes the `<Article><span>…</span></Article>` misclassification (PascalCase component containing text, previously treated as an empty heading via `.toLowerCase()` lookup). - `template-no-invalid-interactive` — fixes `<Article role="button">` / `<Article tabindex={{0}}>` false positives. - `template-no-arguments-for-html-elements` — previously carried inline copy of the lists+scope pattern (established in ember-cli#2689); now uses the shared util. - `template-no-block-params-for-html-elements` — same. This is the rule that introduced the pattern, now deduplicated. Net: 2 inline copies of the canonical pattern dropped; 2 buggy rules fixed; +173 -57 lines.

…+ scope) Remove the inline PascalCase-regex isComponentInvocation heuristic in favor of the lists + scope pattern from ember-cli#2689 / ember-cli#2724. The new lib/utils/is-native-element.js mirrors ember-cli#2724's util byte-for-byte so the two PRs can land in either order without conflict. evaluateChild / evaluateChildren now thread sourceCode through the recursion so the scope check has access to the enclosing template's bindings. Heuristic approaches were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components when shadowed by scope bindings. Treating custom elements as opaque (the same as components) is a behavior improvement — matches ember-cli#2724's convention.

@NullVoxPopuli

…+ scope) Replace the inline PascalCase-regex isComponentInvocation heuristic with the lists + scope pattern established by @NullVoxPopuli in ember-cli#2689 and canonicalised by ember-cli#2724. The new lib/utils/is-native-element.js mirrors without conflict — whichever lands first introduces the file; the other rebases to a no-op on it. Heuristic approaches (PascalCase regex, `tag.includes('.')`, etc.) were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components in GJS/GTS when the name is shadowed by a scope binding (`const div = MyComponent; <div />`). Scope analysis catches this; regex heuristics don't. Call site now uses `!isNativeElement(node, sourceCode)` directly to match the canonical usage pattern in ember-cli#2724's migrated rules.

@NullVoxPopuli

…+ scope) Replace the inline PascalCase-regex isComponentInvocation heuristic with the lists + scope pattern established by @NullVoxPopuli in ember-cli#2689 and canonicalised by ember-cli#2724. The new lib/utils/is-native-element.js mirrors without conflict — whichever lands first introduces the file; the other rebases to a no-op on it. Heuristic approaches (PascalCase regex, `tag.includes('.')`, etc.) were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components in GJS/GTS when the name is shadowed by a scope binding (`const div = MyComponent; <div />`). Scope analysis catches this; regex heuristics don't. Call site now uses `!isNativeElement(node, sourceCode)` directly to match the canonical usage pattern in ember-cli#2724's migrated rules.

…+ scope) Remove the inline PascalCase-regex isComponentInvocation heuristic in favor of the lists + scope pattern from ember-cli#2689 / ember-cli#2724. The new lib/utils/is-native-element.js mirrors ember-cli#2724's util byte-for-byte so the two PRs can land in either order without conflict. evaluateChild / evaluateChildren now thread sourceCode through the recursion so the scope check has access to the enclosing template's bindings. Heuristic approaches were explicitly rejected in ember-cli#2689 because lowercase tags CAN be components when shadowed by scope bindings. Treating custom elements as opaque (the same as components) is a behavior improvement — matches ember-cli#2724's convention.

johanrd marked this pull request as ready for review April 13, 2026 16:10

johanrd force-pushed the day_fix/template-no-block-params-for-html-elements branch from a6eaaf3 to f6dcffa Compare April 13, 2026 17:38

johanrd changed the title ~~Fix template-no-block-params-for-html-elements: align HTML-element detection~~ Post-merge-review: Fix template-no-block-params-for-html-elements: align HTML-element detection Apr 13, 2026

NullVoxPopuli approved these changes Apr 14, 2026

View reviewed changes

NullVoxPopuli requested changes Apr 14, 2026

View reviewed changes

johanrd mentioned this pull request Apr 15, 2026

hbs-parser: register block-param scope for as |x| ember-tooling/ember-eslint-parser#189

Merged

6 tasks

fix(template-no-block-params-for-html-elements): extend allowlist wit…

6308771

…h svg-tags Adds svg-tags to the html-tags allowlist so <circle as |x|> etc. are flagged alongside HTML elements. Documents accepted false negatives (custom elements, namespaced components) via new test cases.

johanrd force-pushed the day_fix/template-no-block-params-for-html-elements branch from 4faabdc to 6308771 Compare April 15, 2026 18:45

johanrd changed the title ~~Post-merge-review: Fix template-no-block-params-for-html-elements: align HTML-element detection~~ Post-merge-review: extend allowlist with svg-tags on template-no-block-params-for-html-elements Apr 15, 2026

johanrd mentioned this pull request Apr 15, 2026

Post-merge-review: Fix template-no-arguments-for-html-elements: add svg and mathml elements #2687

Merged

7 tasks

johanrd requested a review from NullVoxPopuli April 15, 2026 20:16

NullVoxPopuli approved these changes Apr 19, 2026

View reviewed changes

johanrd requested a review from NullVoxPopuli April 19, 2026 18:52

NullVoxPopuli merged commit 367ee0b into ember-cli:master Apr 19, 2026
10 checks passed

NullVoxPopuli added the bug label Apr 19, 2026

github-actions Bot mentioned this pull request Apr 20, 2026

Prepare Release v13.0.0 #2369

Merged

github-actions Bot mentioned this pull request Apr 20, 2026

Prepare Release v14.0.0 johanrd/eslint-plugin-ember#4

Closed

johanrd mentioned this pull request Apr 21, 2026

refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) #2724

Closed

This was referenced Apr 22, 2026

Phase 3 audit — wave findings (18 fixtures, 4 must-fix bugs) johanrd/eslint-plugin-ember#28

Closed

refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) johanrd/eslint-plugin-ember#50

Closed

github-actions Bot mentioned this pull request Apr 24, 2026

Prepare Release v14.0.0 NullVoxPopuli-ai-agent/eslint-plugin-ember#2

Open

johanrd mentioned this pull request Apr 25, 2026

refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) #2746

Merged

github-actions Bot mentioned this pull request Apr 27, 2026

Prepare Release v14.0.0 johanrd/eslint-plugin-ember#61

Open

Uh oh!

Conversation

johanrd commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

NullVoxPopuli left a comment

Choose a reason for hiding this comment

Uh oh!

NullVoxPopuli left a comment

Choose a reason for hiding this comment

Uh oh!

johanrd commented Apr 14, 2026

Uh oh!

NullVoxPopuli commented Apr 14, 2026

Uh oh!

johanrd commented Apr 14, 2026

Uh oh!

NullVoxPopuli commented Apr 15, 2026

Uh oh!

johanrd commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NullVoxPopuli commented Apr 15, 2026

Uh oh!

johanrd commented Apr 15, 2026

Uh oh!

NullVoxPopuli commented Apr 15, 2026

Uh oh!

johanrd commented Apr 15, 2026

Uh oh!

NullVoxPopuli commented Apr 15, 2026

Uh oh!

johanrd commented Apr 15, 2026

Uh oh!

NullVoxPopuli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

johanrd commented Apr 13, 2026 •

edited

Loading

johanrd commented Apr 15, 2026 •

edited

Loading